Skip to content

fix: inherit run formatting when inserting inline structured content (SD-1421)#2501

Open
luccas-harbour wants to merge 3 commits intomainfrom
luccas/sd-1421-bug-when-you-add-sdt-fields-text-should-match-font
Open

fix: inherit run formatting when inserting inline structured content (SD-1421)#2501
luccas-harbour wants to merge 3 commits intomainfrom
luccas/sd-1421-bug-when-you-add-sdt-fields-text-should-match-font

Conversation

@luccas-harbour
Copy link
Contributor

Summary

  • When inserting an inline SDT field in the middle of formatted text, the inserted text now inherits the surrounding font/formatting instead of appearing unstyled
  • Fixes a structural issue where insertStructuredContentInline produced run > structuredContent > run > text nesting — the SDT is now placed at paragraph level between split sibling runs
  • Handles ranged selections correctly by removing the selected text when splitting the parent run

Problem

insertStructuredContentInline used a bare tr.replaceWith(from, to, sdtNode) to insert the SDT. When the cursor was inside a run, ProseMirror placed the SDT as a child of the run (since run has content: 'inline*'). Then wrapTextInRunsPlugin normalized this into run{null} > SDT > run > text, creating unwanted nesting with an empty outer run.

Additionally, the SDT's text content was created without any marks or run wrapping, so it appeared unformatted regardless of the surrounding text style.

Solution

Two changes to insertStructuredContentInline:

  1. Formatting copy — resolves marks and runProperties from the cursor position via getFormattingStateAtPos and wraps the text in a properly formatted run inside the SDT
  2. Run splitting — when the cursor is inside a run, manually splits it into left/right halves and inserts [leftRun, sdtNode, rightRun] at the paragraph level, producing the correct flat structure

@linear
Copy link

linear bot commented Mar 20, 2026

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7747620487

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…ne SDT

- Fall back to plain replaceWith when the selection spans multiple runs,
  instead of only replacing the first run and leaving the rest untouched
- Pass state.storedMarks to getFormattingStateAtPos so pending toolbar
  formatting (e.g. toggling bold before inserting) is respected
Copy link
Contributor

@caiopizzol caiopizzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luccas-harbour nice work — the run splitting and font inheritance logic looks good. both points from the earlier automated review are handled.

two things: the cursor lands in the wrong spot after inserting a field mid-run (line 210), and inserting at the very start of a run isn't tested yet. left inline comments with details. neither blocks the fix for the reported bug, but the cursor one could bite in the template builder.

// surrounding text. The run-split logic below prevents an outer run
// from wrapping the SDT itself.
const runType = schema.nodes.run;
if (runType && !options.json && content.isText) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when options.json is used, this whole block is skipped — the inserted field won't pick up the surrounding font. that's probably intentional, but a comment here would save the next person from wondering if it's a bug.

fragments.push(runType.create(parentRun.attrs, rightContent, parentRun.marks));
}

tr.replaceWith(runStart, runEnd, fragments);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after this replace, the cursor ends up at the end of the text instead of right after the new field. users won't notice if it's triggered by a button, but if they keep typing after inserting a field in the template builder, text goes to the wrong spot. worth fixing with a tr.setSelection(...) after the replace?

@@ -556,6 +556,122 @@ describe('updateStructuredContentByGroup', () => {
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing a test for when the cursor is at the very start of a run — that's the case where there's no text on the left side of the split. worth adding to make sure no empty run sneaks into the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants